Forking: <20230829172828.5qi2pfbladvfgvsg@alvherre.pgsql> Subject: Re: Strange presentaion related to inheritance in \d+ On Tue, Aug 29, 2023 at 07:28:28PM +0200, Alvaro Herrera wrote: > On 2023-Aug-29, Kyotaro Horiguchi wrote: > > > Attached is the initial version of the patch. It prevents "CREATE > > TABLE" from executing if there is an inconsisntent not-null > > constraint. Also I noticed that "ALTER TABLE t ADD NOT NULL c NO > > INHERIT" silently ignores the "NO INHERIT" part and fixed it. > > Great, thank you. I pushed it after modifying it a bit -- instead of > throwing the error in MergeAttributes, I did it in > AddRelationNotNullConstraints(). It seems cleaner this way, mostly > because we already have to match these two constraints there. (I guess > you could argue that we waste catalog-insertion work before the error is > reported and the whole thing is aborted; but I don't think this is a > serious problem in practice.) 9b581c5341 can break dump/restore from old versions, including pgupgrade. postgres=# CREATE TABLE iparent(id serial PRIMARY KEY); CREATE TABLE child (id int) INHERITS (iparent); ALTER TABLE child ALTER id DROP NOT NULL; ALTER TABLE child ADD CONSTRAINT p PRIMARY KEY (id); $ pg_dump -h /tmp -p 5678 postgres -Fc |pg_restore -1 -h /tmp -p 5679 -d postgres ERROR: cannot change NO INHERIT status of inherited NOT NULL constraint "pgdump_throwaway_notnull_0" on relation "child" STATEMENT: ALTER TABLE ONLY public.iparent ADD CONSTRAINT iparent_pkey PRIMARY KEY (id); ALTER TABLE ONLY public.iparent DROP CONSTRAINT pgdump_throwaway_notnull_0; Strangely, if I name the table "parent", it seems to work, which might indicate an ordering/dependency issue. I think there are other issues related to b0e96f3119 (Catalog not-null constraints) - if I dump a v16 server using v17 tools, the backup can't be restored into the v16 server. I'm okay ignoring a line or two like 'unrecognized configuration parameter "transaction_timeout", but not 'syntax error at or near "NO"'. postgres=# CREATE TABLE a(i int not null primary key); $ pg_dump -h /tmp -p 5678 postgres |psql -h /tmp -p 5678 -d new 2024-04-13 21:26:14.510 CDT [475995] ERROR: syntax error at or near "NO" at character 86 2024-04-13 21:26:14.510 CDT [475995] STATEMENT: CREATE TABLE public.a ( i integer CONSTRAINT pgdump_throwaway_notnull_0 NOT NULL NO INHERIT ); ERROR: syntax error at or near "NO" LINE 2: ...er CONSTRAINT pgdump_throwaway_notnull_0 NOT NULL NO INHERIT The other version checks in pg_dump.c are used to construct sql for querying the source db, but this is used to create the sql to restore the target, using syntax that didn't exist until v17. if (print_notnull) { if (tbinfo->notnull_constrs[j][0] == '\0') appendPQExpBufferStr(q, " NOT NULL"); else appendPQExpBuffer(q, " CONSTRAINT %s NOT NULL", fmtId(tbinfo->notnull_constrs[j])); if (tbinfo->notnull_noinh[j]) appendPQExpBufferStr(q, " NO INHERIT"); } This other thread is 6 years old and forgotten again, but still seems relevant. https://www.postgresql.org/message-id/flat/b8794d6a-38f0-9d7c-ad4b-e85adf860fc9%40enterprisedb.com BTW, these comments are out of date: + * In versions 16 and up, we need pg_constraint for explicit NOT NULL + if (fout->remoteVersion >= 170000) + * that we needn't specify that again for the child. (Versions >= 16 no + if (fout->remoteVersion < 170000) -- Justin